Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEPR #16747: deprecate pd.TimeGrouper #16942

Closed
wants to merge 5 commits into from
Closed

DEPR #16747: deprecate pd.TimeGrouper #16942

wants to merge 5 commits into from

Conversation

stevenvandenberghe
Copy link

  • closes DEPR: pd.TimeGrouper  #16747
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry -> Should I do this for a future

@pep8speaks
Copy link

pep8speaks commented Jul 15, 2017

Hello @stevemontana1980! Thanks for updating the PR.

Line 1118:9: E741 ambiguous variable name 'l'

Line 857:5: E731 do not assign a lambda expression, use a def
Line 1558:9: E731 do not assign a lambda expression, use a def
Line 2197:5: E731 do not assign a lambda expression, use a def
Line 2799:5: E731 do not assign a lambda expression, use a def
Line 2870:9: E731 do not assign a lambda expression, use a def
Line 2881:9: E731 do not assign a lambda expression, use a def
Line 3043:9: E731 do not assign a lambda expression, use a def
Line 3085:9: E731 do not assign a lambda expression, use a def

Comment last updated on July 15, 2017 at 12:42 Hours UTC

@imih
Copy link
Contributor

imih commented Jul 15, 2017

#16747

'TimedeltaIndex', 'Timestamp', 'Interval', 'IntervalIndex']

# these are already deprecated; awaiting removal
deprecated_classes = ['WidePanel', 'Panel4D',
'SparseList', 'Expr', 'Term']

# these should be deprecated in the future
deprecated_classes_in_future = ['Panel']
deprecated_classes_in_future = ['Panel', 'TimeGrouper']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think TimeGrouper should go in the deprecated_classes list.

@@ -3030,6 +3030,11 @@ def setup_method(self, method):
self.ts = Series(np.random.randn(1000),
index=date_range('1/1/2000', periods=1000))

def test_TimeGrouper_deprecation(self):
# deprecation TimeGrouper, #16747
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we write these as # GH #16747

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank, changes committed. I'm new at this, so thanks for your patience.

@codecov
Copy link

codecov bot commented Jul 15, 2017

Codecov Report

Merging #16942 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16942      +/-   ##
==========================================
+ Coverage   90.98%   90.98%   +<.01%     
==========================================
  Files         161      161              
  Lines       49306    49307       +1     
==========================================
+ Hits        44859    44860       +1     
  Misses       4447     4447
Flag Coverage Δ
#multiple 88.75% <100%> (ø) ⬆️
#single 40.19% <0%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/resample.py 96.14% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f2b96b...5f044e9. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 15, 2017

Codecov Report

Merging #16942 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16942      +/-   ##
==========================================
- Coverage   90.99%   90.98%   -0.02%     
==========================================
  Files         161      161              
  Lines       49306    49307       +1     
==========================================
- Hits        44868    44860       -8     
- Misses       4438     4447       +9
Flag Coverage Δ
#multiple 88.75% <100%> (ø) ⬆️
#single 40.19% <0%> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/core/resample.py 96.14% <100%> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.71% <0%> (-0.1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f2b96b...5f044e9. Read the comment docs.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • pls add a line in whatsnew (deprecations sections).
  • run tests where this appears locally (e.g. the resample / groupby tests). we need to suppress this error where it is used now. so you need to change any instances of use of TimeGrouper to use Grouper (there are a few). but of course leave the tests on the deprecated one.

@jreback jreback added Deprecate Functionality to remove in pandas Resample resample method labels Jul 15, 2017
@jreback
Copy link
Contributor

jreback commented Aug 18, 2017

can you rebase and update according to comments.

@jreback
Copy link
Contributor

jreback commented Sep 23, 2017

can you rebase / update

@jorisvandenbossche jorisvandenbossche added this to the 0.21.0 milestone Sep 25, 2017
@jreback
Copy link
Contributor

jreback commented Sep 28, 2017

closing in favor of #17703

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Resample resample method
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DEPR: pd.TimeGrouper
6 participants